loader: lazy-load source map cache in CJS loader#62267
loader: lazy-load source map cache in CJS loader#62267gurgunday wants to merge 1 commit intonodejs:mainfrom
Conversation
|
Review requested:
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #62267 +/- ##
==========================================
- Coverage 89.68% 89.68% -0.01%
==========================================
Files 676 676
Lines 206495 206554 +59
Branches 39537 39549 +12
==========================================
+ Hits 185204 185243 +39
- Misses 13435 13450 +15
- Partials 7856 7861 +5
🚀 New features to boost your workflow:
|
| }; | ||
|
|
||
| const { BuiltinModule } = require('internal/bootstrap/realm'); | ||
| const { |
There was a problem hiding this comment.
Usually when you lazy load like this, there's one test that fails if this file is not included anymore in the main path.
Did you run the tests locally? If not, try run this test and see if it fails.
There was a problem hiding this comment.
And if it's not failing, that means lazy loading likely has no effect as it's still included in the snapshot (and loading a module already snapshotted is virtually free)
There was a problem hiding this comment.
Hmm, makes sense. You're right...
|
Closing this one, I wasn't sure about this anyway |
Not sure why we're not lazy loading this? Seems like a good candidate